feat(openai): Set system instruction attribute for Completions API #5359
Conversation
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
| if "blocks" in param_id: | ||
| assert span["data"]["gen_ai.usage.output_tokens"] == 2 | ||
| assert span["data"]["gen_ai.usage.input_tokens"] == 7 | ||
| assert span["data"]["gen_ai.usage.total_tokens"] == 9 | ||
| else: | ||
| assert span["data"]["gen_ai.usage.output_tokens"] == 2 | ||
| assert span["data"]["gen_ai.usage.input_tokens"] == 1 | ||
| assert span["data"]["gen_ai.usage.total_tokens"] == 3 |
There was a problem hiding this comment.
this is not good, but unrelated to this PR.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
alexander-alderman-webb
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| message | ||
| for message in messages | ||
| if not _is_system_instruction_completions(message) | ||
| ] |
There was a problem hiding this comment.
Iterator consumed twice causes silent data loss
Low Severity
In _set_completions_api_input_data, the messages parameter is iterated twice: first in _get_system_instructions_completions(messages) at line 332, then again in the list comprehension at lines 353-357 to build non_system_messages. If messages is a generator or single-use iterator rather than a list, the second iteration would yield no results, silently dropping all non-system messages from telemetry. While OpenAI's SDK typically uses lists, the type annotation accepts Iterable, which permits generators.
There was a problem hiding this comment.
great point, but unfortunately truncate_and_annotate_messages() also assumes that the input is a list.
so the type is correct but our SDK performs poorly in this case. I will tackle this in another PR.
Description
Set the system instruction attribute, conforming to OTtel structure:
https://opentelemetry.io/docs/specs/semconv/registry/attributes/gen-ai/#gen-ai-system-instructions
Issues
Reminders
tox -e linters.feat:,fix:,ref:,meta:)